feat(onbaording): implement basic functions for the new onboarding #17003
Conversation
jrainville
requested review from
micieslak,
caybro,
alexjba and
a team
as code owners
jrainville
changed the title
Jenkins Builds
|
| CreateProfileWithKeycardExistingSeedphrase, | ||
| LoginWithSeedphrase, | ||
| LoginWithSyncing, | ||
| LoginWithKeycard |
There was a problem hiding this comment.
My idea was to use the C++ enum here as the only source of truth (which I created in the other PR which is not merged yet ofc), and also in QML. Perhaps later :)
There was a problem hiding this comment.
Ah yeah. I didn't find an enum that was usable, so I created one, but I have nothing against getting rid of this one to share the same you have once I integrate.
Do you know how to access the enum from Nim?
There was a problem hiding this comment.
Do you know how to access the enum from Nim?
Not exactly but with my slightly limited NIM knowledge, it should be possible 😆
Fixes #16832 Implements all the needed basic Nim functions for the new onboarding. They do no do anything just yet. They shall be integrated in another commit.
jrainville
force-pushed
the
feat/new-onboarding-middle-layer-impl
branch
from
a54d864 to
a0438a9
Compare
| @@ -46,6 +46,6 @@ QtObject { | |||
| } | |||
|
|
|||
| function inputConnectionStringForBootstrapping(connectionString) { | |||
| return root.devicesModule.inputConnectionStringForBootstrapping(connectionString) | |||
| root.devicesModule.inputConnectionStringForBootstrapping(connectionString) | |||
There was a problem hiding this comment.
Is it ok that it doesn't return the value anymore? Not sure if it was ever used, but it affects current behaviour.
There was a problem hiding this comment.
iirc, we never used it in the QML at all, that's why I felt comfortable to remove it
| type PrimaryFlow* {.pure} = enum | ||
| Unknown = 0, | ||
| CreateProfile, | ||
| Login |
There was a problem hiding this comment.
Soo, do we need PrimaryFlow, if all of the values in SecondaryFlow have prefix indicating the primary flow? Looks absolutely redundant 🤔
There was a problem hiding this comment.
Maybe not indeed. I still didn't have access to the C++ enum, so now that I'm rebased on top of it, maybe we can simplify some more
There was a problem hiding this comment.
Seems like you're right. The QML only provides the secondary flow, which is sufficient anyway:
!!! ONBOARDING FINISHED; flow: 1 ; data: {\"password\":\"1234567890\",\"keycardPin\":\"\",\"seedphrase\":\"\",\"enableBiometrics\":false}
I'll get rid of the primary flow complexity
| method inputConnectionStringForBootstrapping*[T](self: Module[T], connectionString: string) = | ||
| self.controller.inputConnectionStringForBootstrapping(connectionString) | ||
|
|
||
| method finishOnboardingFlow*[T](self: Module[T], primaryFlowInt, secondaryFlowInt: int, dataJson: string): string = |
There was a problem hiding this comment.
Nice, I like the simplicity 👍
Fixes #16832
Implements all the needed basic Nim functions for the new onboarding.
They do no do anything just yet. They shall be integrated in another PR.
Everything compiles nonetheless.